AppCred support#1430
Conversation
|
Skipping CI for Draft Pull Request. |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Unable to freeze job graph: Job adoption-standalone-to-crc-ceph-provider depends on openstack-k8s-operators-content-provider which was not run. |
|
recheck |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/fb95ce639e164bb190aa3b41fcda82da ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 59m 19s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/64ab1660f5ff460cb0bdb2682d3b4149 ❌ openstack-k8s-operators-content-provider FAILURE in 15m 15s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5ebbd01a8bc549b2956a87c3507363e2 ❌ openstack-k8s-operators-content-provider FAILURE in 13m 33s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/67ea6f17ebce4477b46d077171537d98 ❌ openstack-k8s-operators-content-provider FAILURE in 13m 49s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/65697c077ffe4630a698b4a94657a08a ❌ openstack-k8s-operators-content-provider FAILURE in 16m 43s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/8b21d132d2c944f4bb8faccfff711e47 ❌ openstack-k8s-operators-content-provider FAILURE in 14m 09s |
Deydra71
force-pushed
the
appcred-support
branch
2 times, most recently
from
62fd3e5 to
27db2bd
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5c42e7939c0a40fa92ed0b1ba8dfae74 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 38s |
| if barbicanSecret == "" { | ||
| barbicanSecret = instance.Spec.Secret | ||
| } | ||
|
|
There was a problem hiding this comment.
I think to prevent the bellow mentioned flapping we'd have to do something like we do for certs to fetch the current auth from the running barbican
if isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Barbican.ApplicationCredential) {
instance.Spec.Barbican.Template.Auth = barbican.Spec.Barbican.Template.Auth
}
| // Set or clear ApplicationCredentialSecret | ||
| // - If AC disabled: use password | ||
| // - If AC enabled AND ready: use AC | ||
| // - If AC enabled BUT not ready: leave unchanged to avoid flapping | ||
| if acSecretName == "" && !isACEnabled(instance.Spec.ApplicationCredential, instance.Spec.Barbican.ApplicationCredential) { | ||
| instance.Spec.Barbican.Template.Auth.ApplicationCredentialSecret = "" | ||
| } else if acSecretName != "" { | ||
| instance.Spec.Barbican.Template.Auth.ApplicationCredentialSecret = acSecretName | ||
| } |
There was a problem hiding this comment.
with the above comment, it might be ok to just assign what EnsureApplicationCredentialForService returns as the ApplicationCredentialSecret
instance.Spec.Barbican.Template.Auth.ApplicationCredentialSecret = acSecretName
- If AC disabled: use password, it returns ""
- If AC enabled AND ready. it returns the name
| ) | ||
| if err != nil { | ||
| return ctrl.Result{}, err | ||
| } |
There was a problem hiding this comment.
do we have to return for the Application Credential not ready yet case when we return with the RequeueAfter?
if (acResult != ctrl.Result{}) {
return acResult, nil
}
| barbicanSecret = instance.Spec.Secret | ||
| } | ||
|
|
||
| acSecretName, acResult, err := EnsureApplicationCredentialForService( |
There was a problem hiding this comment.
the default for AC is that they are disabled. wondering if we don't have to call it if ac is disabled and there is no current barbican.Spec.Barbican.Template.Auth.ApplicationCredentialSecret configured?
| if barbicanReady && (acResult != ctrl.Result{}) { | ||
| return acResult, nil | ||
| } |
There was a problem hiding this comment.
as commented above, wondering why we do it at the bottom and not right after the ensure func?
There was a problem hiding this comment.
I thought that if we returned early right after the rnsure, the service would never get created/udpated when AC is being providioned because we would skip CreateOrPatch?
|
this looks a lot cleaner!! just some questions for clarification, and maybe improving things |
Deydra71
force-pushed
the
appcred-support
branch
2 times, most recently
from
1378a63 to
b14002d
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/137c17c222454d38876bdbcd8e0a819a ❌ openstack-k8s-operators-content-provider FAILURE in 12m 25s |
Deydra71
force-pushed
the
appcred-support
branch
from
b14002d to
abd35b4
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/045a5dddacfa477d9cb68393191d1719 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 52s |
| replace github.com/cert-manager/cmctl/v2 => github.com/cert-manager/cmctl/v2 v2.1.2-0.20241127223932-88edb96860cf //allow-merging | ||
|
|
||
| // appcred related changes | ||
| replace github.com/openstack-k8s-operators/keystone-operator/api => github.com/Deydra71/keystone-operator/api v0.0.0-20260119142142-e3bd4fb9750f //allow-merging |
There was a problem hiding this comment.
Remove these lines after new versions are released and run tidy
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/b1b7e13ddaa04b02a720ca60197ce146 ❌ openstack-k8s-operators-content-provider FAILURE in 12m 34s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/743bfa4caa564529bf798ba240bf1954 ❌ openstack-k8s-operators-content-provider FAILURE in 27m 28s |
|
Note: we need to update cinder, manila and heat after they are bumped, because we changed the placement of |
Deydra71
force-pushed
the
appcred-support
branch
from
9ecd49b to
97e4754
Compare
|
/test openstack-operator-build-deploy-kuttl-4-18 |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/c44a41aa146143b2b8444978acad6ee7 ✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 08m 11s |
|
/retest |
|
/test functional |
|
@Deydra71: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/test openstack-operator-build-deploy-kuttl-4-18 |
Signed-off-by: Veronika Fisarova <vfisarov@redhat.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Deydra71, stuggi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
7 participants
OSPRH-14738
This PR add ApplicationCredential support enabling both global defaults and service-specific overrides in OpenStackControlPlane.
CRD updates:
spec.applicationCredentialsection withenabled,expirationDaysandgracePeriodDaysenabled:falsein every supported service, whileexpirationDaysandgracePeriodDaysare hidden unless specified directly (in that case global values are used).Controller logic:
enable: trueenabledis turned offExample:
In the example barbican is using days overrides, while cinder is using default values.
Depends-On: openstack-k8s-operators/keystone-operator#567